-
Notifications
You must be signed in to change notification settings - Fork 44
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: Move Option and Enumerable to immutables #939
chore: Move Option and Enumerable to immutables #939
Conversation
Enumerable might be leaving the defra repo entirely quite soon - not sure it is worth touching it here. This would also leave us with the |
There wouldn't be an options sub-package. It stays at the root of immutables. It makes more sense to have it here than in client in my opinion. |
You'd prefer an immutables package with a single type, than to either leave it where it is, or to move it to a sub-package in client? And you'd prefer to move enumerable out here, only for it to be moved again pretty soonish? |
Yes. I don't like that client is becoming a catch-all. And option.Option is weird and not idiomatic.
You mean it's going to be move to a separate repo? The work has been done so I don't think it's a big deal if we leave it there until it's taken out. |
Yes, it is to be shared by lens and defra (or less-likely, but possible, removed and replaced from both by a 3rd party lib)
Okay not a problem, so long as we are aware and still happy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
From sourcenetwork/defradb#939 with the licence headers stripped out
214cdef
to
6d3065e
Compare
Codecov Report
@@ Coverage Diff @@
## develop #939 +/- ##
===========================================
- Coverage 60.19% 59.94% -0.25%
===========================================
Files 165 157 -8
Lines 17884 17685 -199
===========================================
- Hits 10766 10602 -164
+ Misses 6155 6129 -26
+ Partials 963 954 -9
|
6d3065e
to
54e8b6b
Compare
Relevant issue(s) Resolves sourcenetwork#854 Resolves sourcenetwork#853 Description This PR adds no new code. It's simply moving or renaming things. Mainly, the Option and Enumerable types are moved the external immutable package and the UpdateEvent is moved to the events package are renamed Update.
Relevant issue(s)
Resolves #854
Resolves #853
Description
This PR adds no new code. It's simply moving or renaming things. Mainly, the
Option
andEnumerable
types are moved theimmutables
package and theUpdateEvent
is moved to theevents
package are renamedUpdate
.Tasks
How has this been tested?
make test
Specify the platform(s) on which this was tested: